-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TRN-808 Sylo Data Pallet #917
base: main
Are you sure you want to change the base?
Conversation
runtime/src/lib.rs
Outdated
pub const MaxResolvers: u32 = 10; | ||
pub const MaxTags: u32 = 10; | ||
pub const MaxEntries: u32 = 100; | ||
pub const MaxServiceEndpoints: u32 = 10; | ||
pub const SyloStringLimit: u32 = 500; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could reduce space for these vars:
pub const MaxResolvers: u32 = 10; | |
pub const MaxTags: u32 = 10; | |
pub const MaxEntries: u32 = 100; | |
pub const MaxServiceEndpoints: u32 = 10; | |
pub const SyloStringLimit: u32 = 500; | |
pub const MaxResolvers: u8 = 10; | |
pub const MaxTags: u8 = 10; | |
pub const MaxEntries: u8 = 100; | |
pub const MaxServiceEndpoints: u8 = 10; | |
pub const SyloStringLimit: u16 = 500; |
runtime/src/weights/pallet_sylo.rs
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
weights need to be generated via CI - the runner would commit the updated weights
@@ -723,3 +723,26 @@ macro_rules! impl_pallet_scheduler_config { | |||
} | |||
}; | |||
} | |||
|
|||
#[macro_export] | |||
macro_rules! impl_pallet_sylo_config { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice; thanks for this
@@ -23,6 +23,7 @@ pallet-evm = { workspace = true } | |||
pallet-assets-ext = { workspace = true } | |||
pallet-dex = { workspace = true } | |||
pallet-futurepass = { workspace = true } | |||
pallet-sylo = { workspace = true } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
may need an entry in the std = [
feature below.
pallet/fee-proxy/src/impls.rs
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will wait for validated E2E tests to approve changes here
pallet/sylo/src/types.rs
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont think we need explicit/restrictive trait-bounds for all the types.
Can take a look at the pub struct SaleInformation<AccountId, BlockNumber> {
in pallet/crowdsale/src/types.rs
file to see struct declarations which just consume a generic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried something like
pub struct ResolverId<Str> {
pub method: Str,
pub identifier: Str,
}
pub trait ToDid {
fn to_did(&self) -> Vec<u8>;
}
impl<T: Get<u32>> ToDid for ResolverId<BoundedVec<u8, T>> {
..
}
but didn't feel like it cleaned up the code, as in the lib implementation, I then had to reference the full type like this ResolverId<BoundedVec<u8, T::StringLimit>>
everywhere
#[pallet::weight({ | ||
T::WeightInfo::set_payment_asset() | ||
})] | ||
pub fn set_payment_asset(origin: OriginFor<T>, payment_asset: AssetId) -> DispatchResult { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
event needs to be emitted for all state changes.
#[pallet::weight({ | ||
T::WeightInfo::set_sylo_resolver_method() | ||
})] | ||
pub fn set_sylo_resolver_method( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here (emit event)
pallet/sylo/src/lib.rs
Outdated
T::WeightInfo::set_payment_asset() | ||
})] | ||
pub fn set_payment_asset(origin: OriginFor<T>, payment_asset: AssetId) -> DispatchResult { | ||
ensure_root(origin)?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rather than using root; use the ApproveOrigin
account (passed in via runtime)
pallet/sylo/src/lib.rs
Outdated
origin: OriginFor<T>, | ||
resolver_method: BoundedVec<u8, T::StringLimit>, | ||
) -> DispatchResult { | ||
ensure_root(origin)?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here (use approved origin)
pallet/sylo/src/lib.rs
Outdated
let mut resolver = | ||
<Resolvers<T>>::get(identifier.clone()).ok_or(Error::<T>::ResolverNotRegistered)?; | ||
|
||
ensure!(who == resolver.controller, Error::<T>::NotController); | ||
|
||
resolver.service_endpoints = service_endpoints.clone(); | ||
|
||
<Resolvers<T>>::insert(identifier.clone(), resolver); | ||
|
||
Self::deposit_event(Event::ResolverUpdated { | ||
id: identifier.to_vec(), | ||
controller: who, | ||
service_endpoints, | ||
}); | ||
|
||
Ok(()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: instead of getting and inserting:
let mut resolver = | |
<Resolvers<T>>::get(identifier.clone()).ok_or(Error::<T>::ResolverNotRegistered)?; | |
ensure!(who == resolver.controller, Error::<T>::NotController); | |
resolver.service_endpoints = service_endpoints.clone(); | |
<Resolvers<T>>::insert(identifier.clone(), resolver); | |
Self::deposit_event(Event::ResolverUpdated { | |
id: identifier.to_vec(), | |
controller: who, | |
service_endpoints, | |
}); | |
Ok(()) | |
<Resolvers<T>>::try_mutate(&identifier, |maybe_resolver| { | |
let resolver = maybe_resolver.as_mut().ok_or(Error::<T>::ResolverNotRegistered)?; | |
ensure!(who == resolver.controller, Error::<T>::NotController); | |
resolver.service_endpoints = service_endpoints.clone(); | |
Self::deposit_event(Event::ResolverUpdated { | |
id: identifier.into_vec(), | |
controller: who, | |
service_endpoints, | |
}); | |
Ok(()) | |
}) |
pallet/sylo/src/lib.rs
Outdated
let resolver = | ||
<Resolvers<T>>::get(identifier.clone()).ok_or(Error::<T>::ResolverNotRegistered)?; | ||
|
||
ensure!(who == resolver.controller, Error::<T>::NotController); | ||
|
||
<Resolvers<T>>::remove(identifier.clone()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: unnecessary clone (memory allocations) can be removed i think
let resolver = | |
<Resolvers<T>>::get(identifier.clone()).ok_or(Error::<T>::ResolverNotRegistered)?; | |
ensure!(who == resolver.controller, Error::<T>::NotController); | |
<Resolvers<T>>::remove(identifier.clone()); | |
let resolver = <Resolvers<T>>::get(&identifier).ok_or(Error::<T>::ResolverNotRegistered)?; | |
ensure!(who == resolver.controller, Error::<T>::NotController); | |
<Resolvers<T>>::remove(&identifier); |
pallet/sylo/src/lib.rs
Outdated
let who = ensure_signed(origin)?; | ||
|
||
ensure!( | ||
<ValidationRecords<T>>::get(who.clone(), data_id.clone()).is_none(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<ValidationRecords<T>>::get(who.clone(), data_id.clone()).is_none(), | |
<ValidationRecords<T>>::get(&who, &data_id).is_none(), |
pallet/sylo/src/lib.rs
Outdated
pub fn validate_sylo_resolvers( | ||
resolvers: BoundedVec<ResolverId<T::StringLimit>, T::MaxResolvers>, | ||
) -> DispatchResult { | ||
let reserved_method = <SyloResolverMethod<T>>::get(); | ||
|
||
// Ensure any sylo data resolvers are already registered | ||
for resolver in resolvers { | ||
if resolver.method == reserved_method { | ||
ensure!( | ||
<Resolvers<T>>::get(resolver.identifier).is_some(), | ||
Error::<T>::ResolverNotRegistered | ||
); | ||
} | ||
} | ||
|
||
Ok(()) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
pub fn validate_sylo_resolvers( | |
resolvers: BoundedVec<ResolverId<T::StringLimit>, T::MaxResolvers>, | |
) -> DispatchResult { | |
let reserved_method = <SyloResolverMethod<T>>::get(); | |
// Ensure any sylo data resolvers are already registered | |
for resolver in resolvers { | |
if resolver.method == reserved_method { | |
ensure!( | |
<Resolvers<T>>::get(resolver.identifier).is_some(), | |
Error::<T>::ResolverNotRegistered | |
); | |
} | |
} | |
Ok(()) | |
} | |
pub fn validate_sylo_resolvers( | |
resolvers: &BoundedVec<ResolverId<T::StringLimit>, T::MaxResolvers>, | |
) -> DispatchResult { | |
let reserved_method = <SyloResolverMethod<T>>::get(); | |
resolvers | |
.iter() | |
.filter(|resolver| resolver.method == reserved_method) | |
.try_for_each(|resolver| { | |
ensure!( | |
<Resolvers<T>>::contains_key(&resolver.identifier), | |
Error::<T>::ResolverNotRegistered | |
); | |
Ok(()) | |
}) | |
} |
pallet/sylo/src/lib.rs
Outdated
Error::<T>::RecordAlreadyCreated | ||
); | ||
|
||
Self::validate_sylo_resolvers(resolvers.clone())?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Self::validate_sylo_resolvers(resolvers.clone())?; | |
Self::validate_sylo_resolvers(&resolvers)?; |
pallet/sylo/src/lib.rs
Outdated
}]), | ||
}; | ||
|
||
<ValidationRecords<T>>::insert(who.clone(), data_id.clone(), record); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<ValidationRecords<T>>::insert(who.clone(), data_id.clone(), record); | |
<ValidationRecords<T>>::insert(&who, &data_id, record); |
pallet/sylo/src/lib.rs
Outdated
) -> DispatchResult { | ||
let who = ensure_signed(origin)?; | ||
|
||
let mut record = <ValidationRecords<T>>::get(who.clone(), data_id.clone()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let mut record = <ValidationRecords<T>>::get(who.clone(), data_id.clone()) | |
let mut record = <ValidationRecords<T>>::get(&who, &data_id) |
pallet/sylo/src/lib.rs
Outdated
let mut record = <ValidationRecords<T>>::get(who.clone(), data_id.clone()) | ||
.ok_or(Error::<T>::RecordNotCreated)?; | ||
|
||
record.entries.force_push(ValidationEntry { | ||
checksum: checksum.clone(), | ||
block: <frame_system::Pallet<T>>::block_number(), | ||
}); | ||
|
||
<ValidationRecords<T>>::insert(who.clone(), data_id.clone(), record); | ||
|
||
Self::deposit_event(Event::ValidationEntryAdded { | ||
author: who, | ||
id: data_id.to_vec(), | ||
checksum, | ||
}); | ||
|
||
Ok(()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
let mut record = <ValidationRecords<T>>::get(who.clone(), data_id.clone()) | |
.ok_or(Error::<T>::RecordNotCreated)?; | |
record.entries.force_push(ValidationEntry { | |
checksum: checksum.clone(), | |
block: <frame_system::Pallet<T>>::block_number(), | |
}); | |
<ValidationRecords<T>>::insert(who.clone(), data_id.clone(), record); | |
Self::deposit_event(Event::ValidationEntryAdded { | |
author: who, | |
id: data_id.to_vec(), | |
checksum, | |
}); | |
Ok(()) | |
<ValidationRecords<T>>::try_mutate(&who, &data_id, |maybe_record| { | |
let record = maybe_record.as_mut().ok_or(Error::<T>::RecordNotCreated)?; | |
record.entries.force_push(ValidationEntry { | |
checksum, | |
block: <frame_system::Pallet<T>>::block_number(), | |
})?; | |
Self::deposit_event(Event::ValidationEntryAdded { | |
author: who, | |
id: data_id.into_vec(), | |
checksum, | |
}); | |
Ok(()) | |
}) |
pallet/sylo/src/lib.rs
Outdated
let mut record = <ValidationRecords<T>>::get(who.clone(), data_id.clone()) | ||
.ok_or(Error::<T>::RecordNotCreated)?; | ||
|
||
if let Some(resolvers) = resolvers.clone() { | ||
Self::validate_sylo_resolvers(resolvers.clone())?; | ||
record.resolvers = resolvers; | ||
} | ||
|
||
if let Some(data_type) = data_type.clone() { | ||
record.data_type = data_type; | ||
} | ||
|
||
if let Some(tags) = tags.clone() { | ||
record.tags = tags; | ||
} | ||
|
||
<ValidationRecords<T>>::insert(who.clone(), data_id.clone(), record); | ||
|
||
Self::deposit_event(Event::ValidationRecordUpdated { | ||
author: who, | ||
id: data_id.to_vec(), | ||
resolvers: resolvers | ||
.map(|resolvers| resolvers.iter().map(|resolver| resolver.to_did()).collect()), | ||
data_type: data_type.map(|data_type| data_type.to_vec()), | ||
tags: tags.map(|tags| tags.iter().map(|tag| tag.to_vec()).collect()), | ||
}); | ||
|
||
Ok(()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
more efficient as we only clone if/when required:
let mut record = <ValidationRecords<T>>::get(who.clone(), data_id.clone()) | |
.ok_or(Error::<T>::RecordNotCreated)?; | |
if let Some(resolvers) = resolvers.clone() { | |
Self::validate_sylo_resolvers(resolvers.clone())?; | |
record.resolvers = resolvers; | |
} | |
if let Some(data_type) = data_type.clone() { | |
record.data_type = data_type; | |
} | |
if let Some(tags) = tags.clone() { | |
record.tags = tags; | |
} | |
<ValidationRecords<T>>::insert(who.clone(), data_id.clone(), record); | |
Self::deposit_event(Event::ValidationRecordUpdated { | |
author: who, | |
id: data_id.to_vec(), | |
resolvers: resolvers | |
.map(|resolvers| resolvers.iter().map(|resolver| resolver.to_did()).collect()), | |
data_type: data_type.map(|data_type| data_type.to_vec()), | |
tags: tags.map(|tags| tags.iter().map(|tag| tag.to_vec()).collect()), | |
}); | |
Ok(()) | |
<ValidationRecords<T>>::try_mutate(&who, &data_id, |maybe_record| { | |
let record = maybe_record.as_mut().ok_or(Error::<T>::RecordNotCreated)?; | |
if let Some(ref new_resolvers) = resolvers { | |
Self::validate_sylo_resolvers(new_resolvers)?; | |
record.resolvers = new_resolvers.clone(); | |
} | |
if let Some(ref new_data_type) = data_type { | |
record.data_type = new_data_type.clone(); | |
} | |
if let Some(ref new_tags) = tags { | |
record.tags = new_tags.clone(); | |
} | |
Self::deposit_event(Event::ValidationRecordUpdated { | |
author: who, | |
id: data_id.into_vec(), | |
resolvers: resolvers.map(|r| r.iter().map(|resolver| resolver.to_did()).collect()), | |
data_type: data_type.map(|dt| dt.into_vec()), | |
tags: tags.map(|t| t.iter().map(|tag| tag.into_vec()).collect()), | |
}); | |
Ok(()) | |
}) |
pallet/sylo/src/lib.rs
Outdated
let who = ensure_signed(origin)?; | ||
|
||
ensure!( | ||
<ValidationRecords<T>>::get(who.clone(), data_id.clone()).is_some(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<ValidationRecords<T>>::get(who.clone(), data_id.clone()).is_some(), | |
<ValidationRecords<T>>::contains_key(&who, &data_id), |
pallet/sylo/src/lib.rs
Outdated
Error::<T>::RecordNotCreated | ||
); | ||
|
||
<ValidationRecords<T>>::remove(who.clone(), data_id.clone()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<ValidationRecords<T>>::remove(who.clone(), data_id.clone()); | |
<ValidationRecords<T>>::remove(&who, &data_id); |
pallet/sylo/src/lib.rs
Outdated
Ok(()) | ||
} | ||
|
||
pub fn payment_asset() -> Option<AssetId> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems redundant; can just call the type function directly IMO
pallet/sylo/src/benchmarking.rs
Outdated
|
||
let mut service_endpoints = BoundedVec::new(); | ||
for _ in 1..q { | ||
service_endpoints.force_push(bounded_string::<T>("https://service-endpoint.one.two.three")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probaby push max string size here too; we need upper bound test
pallet/sylo/src/benchmarking.rs
Outdated
for _ in 1..q { | ||
service_endpoints.force_push(bounded_string::<T>("https://service-endpoint.one.two.three")); | ||
} | ||
}: _(origin::<T>(&alice), identifier, service_endpoints) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
worthwhile having benchmark validations; you can look at other benchmarks (maybe crowdsale pallet) - which has examples of quick validations post-test.
A single validation should do.
pallet/sylo/src/mock.rs
Outdated
type WeightInfo = (); | ||
} | ||
|
||
#[derive(Default)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think we should be able to use the test helpers testext instead od declaring a new/custom one here
pallet/sylo/src/tests.rs
Outdated
} | ||
} | ||
|
||
fn create_and_register_resolver( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move helper functions to top; i think we follow this approach for most of the tests
pallet/sylo/src/tests.rs
Outdated
return (data_id, resolvers, data_type, tags, checksum, record); | ||
} | ||
|
||
fn create_resolver_id(method: &str, identifier: &str) -> ResolverId<<Test as Config>::StringLimit> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dont think this is needed; can inline into the caller function
pallet/sylo/src/tests.rs
Outdated
} | ||
} | ||
|
||
mod validation_records { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: ideally we try create a mod
for each extrinsic - which is easier to reason as we can test all flows within the same module.
Examples of this should be in the crowdsale
or nft
pallet tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice; looking mostly good!
Most of the suggestions are purely syntactical improvements and/or improvements to memory.
Atleast 2 major things are still required:
- benchmark weight updates
- e2e tests to validate sylo token payment (NOTE: the refund for any over-payment would be given in the native currency)
Other than that; logic seems sound.
5a325b7
to
7ed93e4
Compare
7ed93e4
to
42246f2
Compare
30ab16b
to
9865f3e
Compare
9865f3e
to
36b1522
Compare
Description
The PR implements and adds the Sylo Data Verification Pallet to the runtime.
Protocol RFC - https://www.notion.so/futureverse/RFC-Sylo-Data-Verification-Protocol-14a0cb4dab3d808389eddaab37a2c767
The pallet allows users to register and manage resolvers, and also create and manage validation records.
Resolvers
A resolver is intended to be an external service capable of storing and providing off-chain data. The pallet allows a user to register a resolver, which would include an identifier and a list of service endpoints. A resolver is identified by a
ResolverId
, which loosely follows the DID Specification.The
ResolverId
is comprised of two components: amethod
andidentifier
. Themethod
describes what kind of resolver it is. All resolvers registered on chain are considered "sylo" resolvers, and will all use the same reserved string for their resolver method.The account which registers the resolver is considered the controller of the register, and is capable of modifying or removing the resolver later.
Validation Records
The pallet allows a user to create a validation record through the
create_validation_record
extrinsic. A validation record references an externally stored piece of data, and is intended to be used to validate the integrity and authenticity of offchain data.A validation record will have the following attributes:
data_id
- A string representing the id. Validation records are stored in a double map, using thedata_id
as well as the author's account id.author
- The account which created the record. Will have the ability to modify and or delete the record in the future.resolvers
- A list ofResolverIds
which reference how to fetch the data. All resolvers using the reserved sylo method must be registered onchain.data_type
- A string acting as a hint to clients as to what type of data it is.tags
- A list of strings acting as additional metadata informationentries
A list of validation record entry. Each entry represents a distinct version of the data. It includes a checksum for integrity, and also a block number for when the entry was created.Notes
OnChargeTransaction
implementation for the FeeProxy, to detect sylo related extrinsics, and force the swap.Release Notes
Key Changes
Adds the Sylo Data Verification Pallet to the runtime
Type of Change
Extrinsic Changes
Added